Extract RegistrationService; consolidate registration across four controllers - #46
Conversation
…trollers Web registerStore, API register, and both OAuth register() copies had drifted duplicates of the same logic. RegistrationService now owns: - validation rules (shared by web + API) - user creation, invitation acceptance (InvalidInvitationException for controllers to shape into 400/back()->withErrors), federated-domain team-creation rules, OAuth registration (verified email, generated username), the transaction, and the Registered event Controllers shrink to request/response shaping. Future registration hooks (email reputation checks, RFC-001 self-registration routing) now have a single integration point. Also: - Domain::isVerifiedForEmail() replaces four identical private isDomainVerified() methods plus one inline variant - Removed MembershipService (verified unused: never injected or referenced anywhere) Behaviour preserved: full suite green unchanged (1004 tests). One error-path message changes: API OAuth registration failure now surfaces as the generic 'OAuth authentication failed.' 500 instead of 'Unable to register user.' (the service throws instead of returning null).
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov ReportCaution This repository is currently using the Sentry GitHub App to receive Codecov PR comments. This integration will be deprecated on July 8, 2026. Please install the Codecov GitHub App to continue receiving coverage reports on your pull requests. 📢 Thoughts on this report? Let us know! |
2 participants
What
Phase-1 cleanup from the roadmap: the registration logic that had drifted into four near-duplicates (web
registerStore, APIregister, and both OAuthregister()copies) now lives in oneRegistrationService:InvalidInvitationException— each controller shapes it into its own 400 /back()->withErrors), federated-domain team-creation rules, OAuth registration (provider-verified email, unique username generation), transaction ownership, and theRegisteredeventControllers shrink to request/response shaping (net −93 lines). This is also deliberate groundwork: the two upcoming registration-time integrations — email-reputation checks and RFC-001's self-registration routing — now have a single place to plug in instead of four.
Also
Domain::isVerifiedForEmail()replaces four identical privateisDomainVerified()methods plus a fifth inline variant inloginPasswordMembershipService— verified unused (never injected or referenced; 23 dead lines)Behaviour
Preserved — the full suite passes unchanged (1004 tests), which is the point: the existing registration/OAuth/invitation feature tests all exercise the consolidated path. One error-path message differs: API OAuth registration failure surfaces as the generic
OAuth authentication failed.500 rather thanUnable to register user.(the service throws instead of returning null). Pint and PHPStan clean.